Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: Security Domain and SCP11a/b/c features #164

Merged
merged 56 commits into from
Dec 17, 2024
Merged

Conversation

DennisDyallo
Copy link
Collaborator

@DennisDyallo DennisDyallo commented Nov 27, 2024

Description

This PR implements comprehensive support for SCP11a/b/c secure channels and enhances Security Domain functionality. It also enables OATH, OTP, and YubiHsm applications to operate over SCP connections.

Key Changes

  • Introduced new ../Scp/ namespace for improved secure channel management
  • Added support for SCP11a/b/c secure channels
  • Enabled OATH, OTP, and YubiHsm applications over SCP connections
  • Implemented new Security Domain application through SecurityDomainSession

New Components

  • SecurityDomainSession: Application that manages the Security Domain
  • Scp03KeyParameters: SCP03 key configuration
  • Scp11KeyParameters: SCP11 key configuration
  • KeyReference: Refers to keys on the Yubikey Security Domain
  • Scp03State: SCP03 state management
  • Scp11State: SCP11 state management

Deprecation and misc changes

  • Deprecated all classes in ../Scp03/ namespace
  • Replaced with new implementations in ../Scp11/ namespace
  • Modified some class names and functionality to align with Android SDK (yubikit-android) and Python SDK (yubikey-manager)

Migration Notes

The new architecture favors using SecurityDomainSession for Security Domain operations rather than individual command classes. Update existing implementations accordingly.

Type of change

  • Refactor (non-breaking change which improves code quality or performance)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How has this been tested?

Tested using a 5.7.2 key and 5.7.4 FIPS key.

Test configuration:

  • OS version: Win 11
  • Firmware version: 5.4.3, 5.7.2 Non FIPS, 5.7.4 Non FIPS
  • Yubikey model1: 5 MPE, FIPS

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have run dotnet format to format my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

Footnotes

  1. See Yubikey models (Multi-protocol, Security Key, FIPS, Bio, YubiHSM, YubiHSM FIPS)

@DennisDyallo DennisDyallo added the enhancement New feature or request label Nov 27, 2024
@DennisDyallo DennisDyallo self-assigned this Nov 27, 2024
@DennisDyallo DennisDyallo force-pushed the feature/scp11 branch 2 times, most recently from cfa5db9 to 69d6525 Compare November 27, 2024 19:07
feat(scp11): Generate EC Key
feat(sd): Reset Security Domain
misc: Changed static analysis mode
misc: SampleCode projects won't build nuget packages
Store identifiers in dictionary
Utilize current and next TLV values
Improve method functionality
Enhance code readability
update auth-decrypt.md
update auth-sign.md
updated RSA key sizes in user manual files
updated RSA key info in API docs
updated docs and minor adjustments and new tests
support OATH and SCP
support OTP and SCP
support YubiHSM and SCP
throws exception on invalid KeyParameters
add docs
Copy link
Member

@AdamVe AdamVe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me. Thanks for addressing the issues from my comments and fixing the FIPS tests.

clarified docs  (ScpKeyParameters)
removed optional constructor (Scp11KeyParameters)
added test for invalid constructor parameters (Scp11KeyParameters)
fixed test for non fips
hexadecimal notation for key ids in docstring
increased sleep delay for integration test
SCP11[^1] uses asymmetric cryptography based on elliptic curves (NIST P-256) for authentication and key agreement. Compared to SCP03, it uses certificates instead of pre-shared keys, providing greater flexibility in cases where the two entities setting up the secure channel are not deployed in strict pairs. The secure channel can be embedded into complex use cases, such as:
- Installation of payment credentials on wearables
- Production systems
- Remote provisioning of cell phone subscriptions
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure these bullets are really relevant in a YubiKey-centric context.

Copy link
Collaborator Author

@DennisDyallo DennisDyallo Dec 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took them from the GPC SCP11 $2 and thought it may be of value as it describes some of the use cases. The existing document also legitimized the use of SCP by explaining some of the use cases. But since I'm already referencing GPC, perhaps thats enough?

add validation and tests on keyId and keyVersionNumber (KeyReference)
made replaceKvn into an optional parameter
misc: factory method for creating KeyReferences
Changes to be committed:
	modified:   ../docs/index.md
@DennisDyallo DennisDyallo force-pushed the feature/scp11 branch 3 times, most recently from 0a4b319 to f80eca7 Compare December 17, 2024 12:01
fixed xref in whats-new.md
removed unused docs
Copy link

Code Coverage

Package Line Rate Branch Rate Complexity Health
Yubico.Core 44% 32% 4311
Yubico.YubiKey 49% 46% 19680
Summary 48% (32942 / 68517) 43% (8245 / 18990) 23991

Minimum allowed line rate is 40%

@DennisDyallo DennisDyallo merged commit baa3db0 into develop Dec 17, 2024
12 checks passed
@DennisDyallo DennisDyallo deleted the feature/scp11 branch December 17, 2024 13:58
This was referenced Dec 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Development

Successfully merging this pull request may close these issues.

5 participants